Repository navigation
Conversation
Kimchi Code Review
Summary📊 Review Score: 78/100 (overall code quality — 0 lowest, 100 highest) 🧪 Tests: yes — Unit tests were added for firewall NAT rule generation in 📝 Found 4 issue(s). See inline comments for details. What to expectKimchi will analyze the changes in this pull request and post:
The review typically completes within a few minutes. This comment will be updated once the review is ready. Interact with Kimchi
ConfigurationReviews are configured by your organization admin. Powered by Kimchi — AI-powered code review by CAST AI |
c0dccf4 to
480cdd3
Compare
![kimchi-review[bot]](https://avatars.githubusercontent.com/in/3223788?s=60&v=4){kind=small}
There was a problem hiding this comment.
📊 Review Score: 78/100 (overall code quality — 0 lowest, 100 highest)
⏱️ Estimated effort to review: 3/5 (1 = trivial, 5 = very complex)
🧪 Tests: yes — Unit tests were added for firewall NAT rule generation in firewall_test.go and for internal-node route/firewall cleanup in internalnode_k8s_test.go. However, no tests cover the new deleteUnknownSourceIP behavior in the ConfigurationReconciler when NodePort support is disabled.
📝 Found 4 issue(s). See inline comments for details.
e8428e2 to
0f33534
Compare
|
/build |
f65623a to
07b5ed0
Compare
07b5ed0 to
f3290fd
Compare
3c6e54e to
5556f2e
Compare
cheina97
changed the title
cheina97
changed the title
This patch makes provisioning of VPC on all edges optional and based on the the omni.cast.ai/provision-on-all-edge-nodes annotation
Commit 90df75e removed by mistake the generation of the deepcopy files. This commit fixes it and pushes the updated files.
Allow deploying multiple gateway replicas (GatewayServer/GatewayClient CRs) for the same remote cluster. Traffic is distributed across the active gateways using ECMP, while return traffic is forced back through the same gateway using fwmark-based policy routing.
google/nftables#301 fixes the issue of the expr.Ct unmarshalling. So no workaround is needed to compare the fwmark expressions.
* ci: add unit tests on each PR * test: fix flaky * fixup! test: fix flaky
* feat: expose gw name in metrics * feat: update Grafana dashboard to include multiple gw
7e751c1 to
87b0a57
Compare
35abf5a to
7603d1d
Compare
7603d1d to
574a9e2
Compare
This branch has not been deployed
9 participants
Restrict pod
IPremapping to provider VirtualNodes, drop the unknown-source IP mapping controller, and fix the gateway remappingFirewallConfigurationto use a single DNAT chain.Changes
IP mapping controller
Deleted the unknown-source IP controller
Removed
pkg/liqo-controller-manager/ipmapping/configuration_controller.go, its test file, and its suite. The controller-manager no longer creates<cfg>-unknown-sourceIPresources. The correspondingCtrlConfigurationIPMappingconstant and main-manager wiring were removed.Create
IPCRs only for pods on provider VirtualNodesUpdated
OffloadedPodReconciler.Reconcileto look up the Node referenced bypod.Spec.NodeNameand verify it carries the standard virtual-node label (liqo.io/type: virtual-node). Only then does it callCreateOrUpdateIP. Pods running on real local nodes are skipped.Added the required
nodesRBAC (get;list;watch) to the reconciler.External-network remapping
FirewallConfigurationenforceFirewallConfigurationChainsnow allocates only one chain (prerouting / DNAT) instead of two. Removed the postrouting / SNAT chain andensureFirewallConfigurationSNATRules, which caused asymmetric NAT and CRD validation errors (nullchains[1]). The masquerade variant remains unchanged.